In 1753 align create statuses across workflows by jonavellecuerdo · Pull Request #245 · MITLibraries/dspace-submission-composer

jonavellecuerdo · 2026-06-03T18:27:11Z

Purpose and background context

Statuses for item submission post batch creation now comprise of "create_success|failed|skipped", not just "batch_created". This change is to support workflows that require writing item submissions to the DynamoDB table even if an error occurs during batch creation. As a reminder, the opencourseware and sccs workflows will only write item submissions to DynamoDB when no errors occur during the batch creation step, whereas the digitized-theses workflow allows item submissions with status CREATE_FAILED|CREATE_SKIPPED.

The following updates were applied:

Remove default status assignment of CREATE_SUCCESS from base Workflow.create_batch() method
Update OpenCourseWare and SCCS workflow to set statuses during prepare_batch() method

NOTE: The second commit in this PR resolves an issue introduced by the reporting module updates, which unintentionally removed the creation of an errors CSV file, which is still required for the opencourseware and sccs workflows, which do not write records to DynamoDB in the event of a batch creation error. The proposed solution in the second commit is intended to be minimal, and it's worth continuing to reflect on this difference when considering future enhancements to reporting.

How can a reviewer manually see the effects of these changes?

For now, a review of code changes is sufficient.

Includes new or updated dependencies?

NO

Changes expectations for external applications?

NO

What are the relevant tickets?

https://mitlibraries.atlassian.net/browse/IN-1753

Code review

Code review best practices are documented here and you are encouraged to have a constructive dialogue with your reviewers about their preferences and expectations.

Why these changes are being introduced: * Statuses for item submission post batch creation now comprise of "create_success|failed|skipped", not just "batch_created". This change is to support workflows that require writing item submissions to the DynamoDB table even if an error occurs during batch creation. How this addresses that need: * Remove references to ItemSubmissionStatus.BATCH_CREATED Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-1753

coveralls · 2026-06-03T18:29:37Z

Coverage Report for CI Build 27027257687

Warning

No base build found for commit a73b9e2 on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 83.438%

Details

Patch coverage: 3 uncovered changes across 1 file (27 of 30 lines covered, 90.0%).

Uncovered Changes

File	Changed	Covered	%
dsc/cli.py	4	1	25.0%
Total (4 files)	30	27	90.0%

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats


Relevant Lines:	2065
Covered Lines:	1723
Line Coverage:	83.44%
Coverage Strength:	0.83 hits per line

💛 - Coveralls

Copilot

Pull request overview

Aligns “create” step item-submission statuses across workflows by replacing the legacy batch_created status with explicit create_success|create_failed|create_skipped, and restores create-step reporting of batch-creation errors via an errors.csv attachment.

Changes:

Remove BATCH_CREATED and update create-step status expectations to CREATE_SUCCESS across workflows/tests.
Stop the base workflow from overwriting statuses during _create_batch_in_db, requiring workflows to set create statuses during prepare_batch().
Reintroduce create-step error reporting by allowing create reports to attach an errors.csv when batch creation fails.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`dsc/db/models.py`	Removes `BATCH_CREATED` enum value to enforce the new create-status scheme.
`dsc/workflows/base/workflow.py`	Stops default status assignment in `_create_batch_in_db`; extends `send_report()` to pass optional report kwargs.
`dsc/workflows/simple_csv/workflow.py`	Sets `CREATE_SUCCESS` during `prepare_batch()` for SimpleCSV-derived workflows.
`dsc/workflows/opencourseware/workflow.py`	Sets `CREATE_SUCCESS` during `prepare_batch()` for OpenCourseWare workflow.
`dsc/reports/base.py`	Adds optional `errors.csv` attachment for create reports; skips attachments whose generators return `None`.
`dsc/cli.py`	Passes batch-creation errors into the create report when batch creation fails.
`tests/conftest.py`	Renames DynamoDB fixture and updates seeded records/statuses to `CREATE_SUCCESS`.
`tests/test_cli.py`	Updates fixture name and replaces `BATCH_CREATED` expectations with `CREATE_SUCCESS`.
`tests/test_db.py`	Updates fixture name for DynamoDB-backed tests.
`tests/test_item_submission.py`	Updates fixture name and replaces `BATCH_CREATED` expectations with `CREATE_SUCCESS`.
`tests/test_workflow_base.py`	Updates fixture name and replaces `BATCH_CREATED` expectations with `CREATE_SUCCESS`.
`tests/test_workflow_archivesspace.py`	Updates fixture name for DynamoDB-backed workflow tests.
`tests/test_workflow_opencourseware.py`	Updates fixture name and adds `CREATE_SUCCESS` to expected prepared submissions.
`tests/test_workflow_simple_csv.py`	Adds `CREATE_SUCCESS` to expected prepared submissions.

Comments suppressed due to low confidence (1)

dsc/reports/base.py:47

Report.__init__/Report.load annotate **kwargs as list, which implies every kwarg value must be a list. In practice errors may be None, and future report kwargs may not be lists. Loosen the kwarg value type to avoid misleading type hints (and potential static-analysis noise).

    def __init__(self, workflow_name: str, batch_id: str, **kwargs: list):
        self.workflow_name = workflow_name
        self.batch_id = batch_id
        self.report_date = datetime.now(tz=UTC).strftime("%Y%m%dT%H%M%SZ")

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ehanson8

Question about kwargs

ehanson8

Great work!

Why these changes are being introduced: * The digitized theses workflow allows for writing records to DynamoDB in cases whereas other workflows (OpenCourseWare and SCCS) would normally raise a BatchCreationFailedError. When a workflow skips writing to DynamoDB because of a batch creation error, the 'create' reporting module needs a way to access errors. NOTE: This returns the creation of an errors CSV file with the new reporting structure. How this addresses that need: * Workflow.send_report accepts kwargs, allowing assignment of errors during `create` step * Add 'errors' attribute to base `Report` class * Allow attachment method to return None Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-1753

jonavellecuerdo marked this pull request as ready for review June 3, 2026 18:56

jonavellecuerdo requested a review from a team as a code owner June 3, 2026 18:56

jonavellecuerdo requested a review from ehanson8 June 4, 2026 13:55

ehanson8 requested a review from Copilot June 4, 2026 14:58

Copilot started reviewing on behalf of ehanson8 June 4, 2026 14:58 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Comment thread dsc/cli.py

Comment thread dsc/workflows/base/workflow.py Outdated

Comment thread dsc/reports/base.py Outdated

ehanson8 reviewed Jun 4, 2026

View reviewed changes

Comment thread tests/conftest.py

Comment thread dsc/reports/base.py Outdated

jonavellecuerdo added a commit that referenced this pull request Jun 5, 2026

Address comments in PR #245

cb8cbae

jonavellecuerdo requested a review from ehanson8 June 5, 2026 14:53

ehanson8 approved these changes Jun 5, 2026

View reviewed changes

jonavellecuerdo force-pushed the IN-1753-align-create-statuses-across-workflows branch from cb8cbae to e91a29f Compare June 5, 2026 16:33

jonavellecuerdo merged commit ced9ae1 into main Jun 5, 2026
6 checks passed

jonavellecuerdo deleted the IN-1753-align-create-statuses-across-workflows branch June 8, 2026 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

In 1753 align create statuses across workflows#245

In 1753 align create statuses across workflows#245
jonavellecuerdo merged 2 commits into
mainfrom
IN-1753-align-create-statuses-across-workflows

jonavellecuerdo commented Jun 3, 2026 •

edited

Loading

Uh oh!

coveralls commented Jun 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ehanson8 left a comment

Uh oh!

Uh oh!

Uh oh!

ehanson8 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

jonavellecuerdo commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose and background context

How can a reviewer manually see the effects of these changes?

Includes new or updated dependencies?

Changes expectations for external applications?

What are the relevant tickets?

Code review

Uh oh!

coveralls commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 27027257687

Coverage: 83.438%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ehanson8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ehanson8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jonavellecuerdo commented Jun 3, 2026 •

edited

Loading

coveralls commented Jun 3, 2026 •

edited

Loading